Partition_2: fix optimal_convex_partition_2 producing non-simple output for certain starting vertices#9349
Open
RajdeepKushwaha5 wants to merge 1 commit intoCGAL:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a robustness bug in CGAL::optimal_convex_partition_2 where certain choices of the input polygon’s starting vertex could lead to invalid (non-simple / non-convex) partition pieces, and adds a regression test covering all rotations of the reported failing polygon.
Changes:
- Normalize the internal polygon representation by rotating it so the lexicographically smallest vertex is at index 0 before running the optimal convex partition algorithm.
- Add a regression test case (cup-shaped octagon from #9322) and validate all 8 cyclic rotations produce only simple and convex pieces.
- Introduce small test helpers (
make_cup_octagon_issue_9322,partition_is_valid) to support the regression test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
Partition_2/include/CGAL/Partition_2/partition_optimal_convex_2.h |
Rotates internal polygon to a stable/convex anchor vertex to prevent preprocessing corruption and invalid output. |
Partition_2/test/Partition_2/convex_test_polys.h |
Adds a helper to build the issue #9322 cup-shaped octagon with configurable rotation. |
Partition_2/test/Partition_2/test_optimal_convex.h |
Adds a regression loop over all rotations and validates partition pieces are simple and convex. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+540
to
+548
| for (typename P_Polygon_2::iterator it = std::next(polygon.begin()); | ||
| it != polygon.end(); ++it) | ||
| { | ||
| if (less_xy(static_cast<typename Traits::Point_2>(*it), | ||
| static_cast<typename Traits::Point_2>(*min_it))) | ||
| min_it = it; | ||
| } | ||
| std::rotate(polygon.begin(), min_it, polygon.end()); | ||
| } |
There was a problem hiding this comment.
std::rotate is used here, but this header does not include <algorithm>. Relying on transitive includes is non-portable and can break compilation on some standard library implementations; please add the proper include in this header.
…tain start vertices The algorithm decomposes the polygon from vertex 0 to vertex n-1, treating the boundary edge (n-1 -> 0) as the 'closing' edge of the outermost sub-problem. During preprocessing, the validity of this closing edge is stored in edges[n-1][0] (lower triangular), while the visibility graph additionally marks edges[0][n-1] (upper triangular) as visible and computes its validity as if it were an interior diagonal. When vertex 0 is at a position where these two characterisations conflict -- concretely, when vertex 0 is not a convex vertex of the polygon -- the sub-problem boundaries are mis-classified and the diagonal insertion step can produce non-simple or non-convex output pieces. Fix: before running the algorithm, rotate the internal polygon representation so that the lexicographically smallest vertex is at index 0. For any simple polygon the lex-min vertex is guaranteed to be a convex vertex, satisfying the algorithm's anchor assumption and eliminating the inconsistency. The rotation is applied to the Partitioned_polygon_2 vector before any diagonal lists are populated, so std::rotate is safe and the output geometry (the actual convex pieces) is unaffected. A regression test is added that exercises all 8 rotations of the cup-shaped octagon from the bug report; the originally failing rotations 0 (start at (4,4)), 1 (start at (6,6)), and 2 (start at (0,6)) are included. Fixes CGAL#9322
RajdeepKushwaha5
force-pushed
the
fix/partition-2-optimal-convex-starting-vertex
branch
from
b364cd7 to
c7915f4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9322
Problem
CGAL::optimal_convex_partition_2produced non-simple (and non-convex) output polygons when the input polygon was supplied with certain start vertices.Root Cause
The underlying algorithm decomposes the polygon from vertex 0 to vertex n-1. During preprocessing the closing boundary edge (n-1 → 0) and the candidate interior diagonal edge (0, n-1) are stored in conflicting slots when vertex 0
is not convex, corrupting the diagonal selection.
Fix
Before running the algorithm, rotate the internal polygon so that the lexicographically smallest vertex is at index 0. The lex-min vertex of any simple polygon is guaranteed to be convex.
Tests Added
make_cup_octagon_issue_9322helper + loop over all 8 rotations asserting every output piece is simple and convex.